Reduce spin-waiting in Xen serial driver:

 1. Split the serial port lock into receiver and transmitter locks.
 2. In the ns16550 interrupt, only call the generic serial service
    routines if there is receive (or transmit) work to do.
 3. In the generic transmit ISR, avoid long spin-waits by *trying*
    to take the transmitter lock and, if that fails, check again
    whether the transmitter is empty. This will allow us to bail
    bail quickly if there is a long-term lock holder stuffing lots
    of bytes.

Also, gdbstub should be setting its serial handle in synchronous mode,
just for sanity.

Signed-off-by: Keir Fraser <keir@xensource.com>

diff --git a/xen/common/gdbstub.c b/xen/common/gdbstub.c
index fe51f79d353f1ad6e1b00219a3da5469fffa5b8d..89341229258ae2897f0422cfc45b5e7f56c26a4a 100644 (file)
--- a/xen/common/gdbstub.c
+++ b/xen/common/gdbstub.c
@@ -562,6 +562,7 @@ initialise_gdb(void)
     gdb_ctx->serhnd = serial_parse_handle(opt_gdb);
     if ( gdb_ctx->serhnd != -1 )
         printk("GDB stub initialised.\n");
+    serial_start_sync(gdb_ctx->serhnd);
 }
 
 /*

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 9476f473a2705033fde5c470608cae0a4325539e..f1ad820f0d26153c38187f4f5836add83c66a4dd 100644 (file)
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -121,8 +121,11 @@ static void ns16550_interrupt(
 
     while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
     {
-        serial_tx_interrupt(port, regs);
-        serial_rx_interrupt(port, regs);
+        char lsr = ns_read_reg(uart, LSR);
+        if ( lsr & LSR_THRE )
+            serial_tx_interrupt(port, regs);
+        if ( lsr & LSR_DR )
+            serial_rx_interrupt(port, regs);
     }
 }
 

diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
index 5eb0589443ea18f015c51f38c80114c29357295f..1cb50c0d94bb6112f491f7ffe9f56cc6266392ce 100644 (file)
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -7,6 +7,7 @@
  */
 
 #include <xen/config.h>
+#include <xen/delay.h>
 #include <xen/init.h>
 #include <xen/irq.h>
 #include <xen/keyhandler.h> 
@@ -15,8 +16,8 @@
 #include <xen/serial.h>
 
 static struct serial_port com[2] = {
-    { .lock = SPIN_LOCK_UNLOCKED }, 
-    { .lock = SPIN_LOCK_UNLOCKED }
+    { .rx_lock = SPIN_LOCK_UNLOCKED, .tx_lock = SPIN_LOCK_UNLOCKED }, 
+    { .rx_lock = SPIN_LOCK_UNLOCKED, .tx_lock = SPIN_LOCK_UNLOCKED }
 };
 
 void serial_rx_interrupt(struct serial_port *port, struct cpu_user_regs *regs)
@@ -25,7 +26,7 @@ void serial_rx_interrupt(struct serial_port *port, struct cpu_user_regs *regs)
     serial_rx_fn fn = NULL;
     unsigned long flags;
 
-    spin_lock_irqsave(&port->lock, flags);
+    spin_lock_irqsave(&port->rx_lock, flags);
 
     if ( port->driver->getc(port, &c) )
     {
@@ -39,7 +40,7 @@ void serial_rx_interrupt(struct serial_port *port, struct cpu_user_regs *regs)
             port->rxbuf[MASK_SERIAL_RXBUF_IDX(port->rxbufp++)] = c;            
     }
 
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->rx_lock, flags);
 
     if ( fn != NULL )
         (*fn)(c & 0x7f, regs);
@@ -50,7 +51,19 @@ void serial_tx_interrupt(struct serial_port *port, struct cpu_user_regs *regs)
     int i;
     unsigned long flags;
 
-    spin_lock_irqsave(&port->lock, flags);
+    local_irq_save(flags);
+
+    /*
+     * Avoid spinning for a long time: if there is a long-term lock holder
+     * then we know that they'll be stuffing bytes into the transmitter which
+     * will therefore not be empty for long.
+     */
+    while ( !spin_trylock(&port->tx_lock) )
+    {
+        if ( !port->driver->tx_empty(port) )
+            return;
+        cpu_relax();
+    }
 
     if ( port->driver->tx_empty(port) )
     {
@@ -63,7 +76,7 @@ void serial_tx_interrupt(struct serial_port *port, struct cpu_user_regs *regs)
         }
     }
 
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->tx_lock, flags);
 }
 
 static void __serial_putc(struct serial_port *port, char c)
@@ -117,7 +130,7 @@ void serial_putc(int handle, char c)
     if ( (handle == -1) || !port->driver || !port->driver->putc )
         return;
 
-    spin_lock_irqsave(&port->lock, flags);
+    spin_lock_irqsave(&port->tx_lock, flags);
 
     if ( (c == '\n') && (handle & SERHND_COOKED) )
         __serial_putc(port, '\r');
@@ -129,7 +142,7 @@ void serial_putc(int handle, char c)
 
     __serial_putc(port, c);
 
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->tx_lock, flags);
 }
 
 void serial_puts(int handle, const char *s)
@@ -141,7 +154,7 @@ void serial_puts(int handle, const char *s)
     if ( (handle == -1) || !port->driver || !port->driver->putc )
         return;
 
-    spin_lock_irqsave(&port->lock, flags);
+    spin_lock_irqsave(&port->tx_lock, flags);
 
     while ( (c = *s++) != '\0' )
     {
@@ -156,7 +169,7 @@ void serial_puts(int handle, const char *s)
         __serial_putc(port, c);
     }
 
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->tx_lock, flags);
 }
 
 char serial_getc(int handle)
@@ -168,27 +181,28 @@ char serial_getc(int handle)
     if ( (handle == -1) || !port->driver || !port->driver->getc )
         return '\0';
 
-    do {        
+    do {
         for ( ; ; )
         {
-            spin_lock_irqsave(&port->lock, flags);
+            spin_lock_irqsave(&port->rx_lock, flags);
             
             if ( port->rxbufp != port->rxbufc )
             {
                 c = port->rxbuf[MASK_SERIAL_RXBUF_IDX(port->rxbufc++)];
-                spin_unlock_irqrestore(&port->lock, flags);
+                spin_unlock_irqrestore(&port->rx_lock, flags);
                 break;
             }
             
             if ( port->driver->getc(port, &c) )
             {
-                spin_unlock_irqrestore(&port->lock, flags);
+                spin_unlock_irqrestore(&port->rx_lock, flags);
                 break;
             }
 
-            spin_unlock_irqrestore(&port->lock, flags);
+            spin_unlock_irqrestore(&port->rx_lock, flags);
 
             cpu_relax();
+            udelay(100);
         }
     } while ( ((handle & SERHND_LO) &&  (c & 0x80)) ||
               ((handle & SERHND_HI) && !(c & 0x80)) );
@@ -241,7 +255,7 @@ void serial_set_rx_handler(int handle, serial_rx_fn fn)
     if ( handle == -1 )
         return;
 
-    spin_lock_irqsave(&port->lock, flags);
+    spin_lock_irqsave(&port->rx_lock, flags);
 
     if ( port->rx != NULL )
         goto fail;
@@ -265,11 +279,11 @@ void serial_set_rx_handler(int handle, serial_rx_fn fn)
         port->rx = fn;
     }
 
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->rx_lock, flags);
     return;
 
  fail:
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->rx_lock, flags);
     printk("ERROR: Conflicting receive handlers for COM%d\n", 
            handle & SERHND_IDX);
 }
@@ -277,8 +291,13 @@ void serial_set_rx_handler(int handle, serial_rx_fn fn)
 void serial_force_unlock(int handle)
 {
     struct serial_port *port = &com[handle & SERHND_IDX];
-    if ( handle != -1 )
-        port->lock = SPIN_LOCK_UNLOCKED;
+
+    if ( handle == -1 )
+        return;
+
+    port->rx_lock = SPIN_LOCK_UNLOCKED;
+    port->tx_lock = SPIN_LOCK_UNLOCKED;
+
     serial_start_sync(handle);
 }
 
@@ -290,7 +309,7 @@ void serial_start_sync(int handle)
     if ( handle == -1 )
         return;
     
-    spin_lock_irqsave(&port->lock, flags);
+    spin_lock_irqsave(&port->tx_lock, flags);
 
     if ( port->sync++ == 0 )
     {
@@ -303,7 +322,7 @@ void serial_start_sync(int handle)
         }
     }
 
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->tx_lock, flags);
 }
 
 void serial_end_sync(int handle)
@@ -314,11 +333,11 @@ void serial_end_sync(int handle)
     if ( handle == -1 )
         return;
     
-    spin_lock_irqsave(&port->lock, flags);
+    spin_lock_irqsave(&port->tx_lock, flags);
 
     port->sync--;
 
-    spin_unlock_irqrestore(&port->lock, flags);
+    spin_unlock_irqrestore(&port->tx_lock, flags);
 }
 
 int serial_tx_space(int handle)

diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index 28f11f4677268014c7d8c04f0511ab757187379b..d8426630fc7fa5876d08f0ee4338656ec5187861 100644 (file)
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -42,7 +42,7 @@ struct serial_port {
     char                rxbuf[SERIAL_RXBUFSZ];
     unsigned int        rxbufp, rxbufc;
     /* Serial I/O is concurrency-safe. */
-    spinlock_t          lock;
+    spinlock_t          rx_lock, tx_lock;
 };
 
 struct uart_driver {